Track array value type overwrites in by-reference `foreach` without key variable by phpstan-bot · Pull Request #5542 · phpstan/phpstan-src

phpstan-bot · 2026-04-26T18:24:59Z

Summary

When iterating an array by reference without a key variable (foreach ($arr as &$v) { $v = X; }), PHPStan did not track that the array's element type was being overwritten. The same construct WITH a key variable (foreach ($arr as $k => &$v) { $v = X; }) already worked correctly.

This PR extends the post-loop array type rewrite mechanism in NodeScopeResolver to also handle the by-reference-without-key case.

Changes

src/Analyser/NodeScopeResolver.php: Extended the foreach post-loop array type rewrite (around line 1293-1475) to handle $stmt->byRef && $stmt->keyVar === null:
- Added $byRefWithoutKey flag detection
- Extended initial scope collection to use OriginalForeachValueExpr (value was overwritten) instead of OriginalForeachKeyExpr (key was not modified)
- Extended continue exit point handling with the same OriginalForeachValueExpr logic
- Added a branch inside the rewrite block that reads the value variable's type directly (instead of reading from ArrayDimFetch($expr, $keyVar))
- Relaxed the outer condition from $stmt->keyVar !== null to $stmt->keyVar !== null || $byRefWithoutKey
Updated test assertions in existing files that were asserting the old imprecise behavior:
- tests/PHPStan/Analyser/nsrt/overwritten-arrays.php — doFoo7: array<int, 1|string> → array<int, 1>
- tests/PHPStan/Analyser/nsrt/bug-13809.php — foo(): list<mixed> → list<'foo'>, bar3(): list<mixed> → list<'foo'|'maybe'>, baz(): list<string> → list<'bar'>
- tests/PHPStan/Analyser/nsrt/bug-14083.php — example(): list<string> → list<uppercase-string>
- tests/PHPStan/Analyser/nsrt/bug-14084.php — example() and example2(): list<string> → list<uppercase-string>
- tests/PHPStan/Rules/Variables/data/bug-14124.php — example3a(): list<string> → list<uppercase-string>
- tests/PHPStan/Rules/Variables/data/bug-14124b.php — example3a(): list<list<string>> → list<list<uppercase-string>>

Root cause

The post-loop array type rewrite in NodeScopeResolver was gated on $stmt->keyVar !== null. This gate exists because the mechanism relies on:

Checking OriginalForeachKeyExpr to verify the key wasn't modified
Reading the type of $arr[$key] from the final scope to determine the new element type

Without a key variable, neither of these steps is possible. The fix adds a parallel mechanism for the by-reference case: it uses OriginalForeachValueExpr (already tracked for other purposes) to verify the value was overwritten, and reads the value variable's type directly instead of through an ArrayDimFetch.

Test

tests/PHPStan/Analyser/nsrt/bug-1940.php: Comprehensive regression test covering:
- Basic by-ref overwrite without key
- Conditional overwrite (only some iterations)
- Always overwritten via if/else branches
- List types, string keys, non-empty arrays
- Break (should NOT rewrite — not all elements visited)
- Continue with full overwrite, continue with branch overwrites, continue with partial overwrite
- Property as iteratee ($this->prop)
- Function-based transformation (intval)
- No overwrite at all (read-only loop)

Fixes phpstan/phpstan#1940

VincentLanglet

I don't understand how it's related to the fact it's "by ref without key" since the same issue exists when a key is used

foreach ($a as $k => &$testArray) {
	$testArray['two'] = 'two';
}

https://phpstan.org/r/b0dafda8-0a2f-4389-95c9-64a00039b0ff

phpstan-bot · 2026-04-26T20:47:17Z

I processed this review but have nothing to report.

VincentLanglet · 2026-04-27T16:15:23Z

+					$arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes);
+
+					$valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType());
+				} elseif ($stmt->keyVar !== null) {


You can keep the same if order

if $stmt->keyVar !== null

elseif byRefWithoutKey

Done. Swapped the if/elseif branch order so $stmt->keyVar !== null is checked first and $byRefWithoutKey is the elseif, matching the reviewer's suggestion. All tests pass and PHPStan self-analysis is clean.

VincentLanglet

I don't understand why it was working for ref overwrite when a key variable is used. Please explain me.

The fact a key is used or not shouldn't impact the value

phpstan-bot · 2026-04-27T16:23:06Z

Now let me address the reviewer's actual question. They want to understand why the with-key case already worked but the without-key case didn't. This is an explanation question, not a code change request. Here's the answer:

The reviewer's question is: why does the presence/absence of a key variable affect whether value type overwrites are tracked?

The answer is that the key variable isn't used to track the key's type — it's the mechanism by which PHPStan reads back the overwritten value type from the array after the loop body executes.

Here's how the with-key case works:

In foreach ($arr as $k => &$v), PHPStan constructs the expression $arr[$k] (an ArrayDimFetch node)
After the loop body runs, it evaluates $scope->getType($arr[$k]) in the final scope
Since $v is a reference to $arr[$k], any assignment $v = X also updates $arr[$k]'s type in the scope
So reading $arr[$k] gives the overwritten type, which PHPStan uses to rewrite the array's value type

Without a key variable (foreach ($arr as &$v)), there's no $k to build the ArrayDimFetch expression — you can't construct $arr[$k] when $k doesn't exist. The entire mechanism relied on that expression as the probe for the new element type.

The fix adds a parallel path for the keyless case: instead of reading $arr[$k], it reads $v directly (since $v is the value variable that was assigned). Both approaches get to the same answer — the overwritten element type — but via different expressions.

The if ordering has been swapped per the reviewer's suggestion (already pushed as f72b536b9).

VincentLanglet

Add non regression test for issue 1311
phpstan/phpstan#1311

class HelloWorld
{
	/**
	 * @var array<int, string>
	 */
	private $list = [];
	
	public function convertList(): void
	{
		$temp = [1, 2, 3];
		
		foreach ($temp as &$item) {
			$item = (string) $item;
		}
		
		$this->list = $temp;
	}
}

(new HelloWorld())->convertList();

phpstan-bot · 2026-04-27T16:41:07Z

Done. Added a non-regression test for phpstan/phpstan#1311 in tests/PHPStan/Analyser/nsrt/bug-1311.php. The test verifies that a by-reference foreach without key variable correctly tracks type overwrites when casting values ($item = (string) $item on an array<int, int> produces array<int, lowercase-string&numeric-string&uppercase-string>). All tests pass and PHPStan self-analysis is clean.

…ey variable - Extend post-loop array type rewrite in NodeScopeResolver to handle `foreach ($arr as &$v)` (without `$key =>`) by tracking the value variable's type via `OriginalForeachValueExpr` instead of relying on `OriginalForeachKeyExpr` + `ArrayDimFetch` - When the value is always overwritten (OriginalForeachValueExpr absent in all paths), replace the array's value type with the final value variable type — matching the existing behavior for the key-based case - Handle continue exit points for the by-ref without key case with the same logic: check OriginalForeachValueExpr presence in each continue scope - Update assertions in existing tests (bug-13809, bug-14083, bug-14084, bug-14124, bug-14124b, overwritten-arrays) that were asserting the old imprecise behavior

…works The reviewer pointed out that modifying array sub-elements through a by-reference foreach with a key variable also had issues. Testing confirms this case already works correctly on the current branch — both with and without a key variable. The reviewer's example was likely tested against the stable phpstan.org release which runs an older version. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Per reviewer feedback, keep the original branch order so the existing keyVar path comes first and the new byRefWithoutKey path is the elseif. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Verifies that by-reference foreach without key variable correctly tracks type overwrites when casting values (e.g. `$item = (string) $item`). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

staabm

a lot is going on here and I cannot say I understand all the changes/details.

test results look great though. if you feel confident, please go ahead.

…ithout key variable (#5542)" This reverts commit d883cd8.

ondrejmirtes · 2026-04-28T08:28:51Z

Reverted

…without key variable (phpstan#5542)" This reverts commit 6c69c44.

VincentLanglet reviewed Apr 26, 2026

View reviewed changes

VincentLanglet reviewed Apr 27, 2026

View reviewed changes

VincentLanglet approved these changes Apr 27, 2026

View reviewed changes

VincentLanglet requested a review from staabm April 27, 2026 17:25

VincentLanglet and others added 4 commits April 28, 2026 07:27

Reorder if/elseif: check keyVar !== null before byRefWithoutKey

2f8f531

Per reviewer feedback, keep the original branch order so the existing keyVar path comes first and the new byRefWithoutKey path is the elseif. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add non-regression test for phpstan/phpstan#1311

0c39707

Verifies that by-reference foreach without key variable correctly tracks type overwrites when casting values (e.g. `$item = (string) $item`). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

staabm force-pushed the create-pull-request/patch-vwbkpoi branch from 9b3d9ad to 0c39707 Compare April 28, 2026 05:28

staabm approved these changes Apr 28, 2026

View reviewed changes

VincentLanglet merged commit d883cd8 into phpstan:2.1.x Apr 28, 2026
655 of 659 checks passed

VincentLanglet deleted the create-pull-request/patch-vwbkpoi branch April 28, 2026 07:42

phpstan-bot mentioned this pull request Apr 28, 2026

Array iterated over by ref does not understand it's being overwritten phpstan/phpstan#1940

Open

ondrejmirtes added a commit that referenced this pull request Apr 28, 2026

Revert "Track array value type overwrites in by-reference foreach w…

6c69c44

…ithout key variable (#5542)" This reverts commit d883cd8.

phpstan-bot mentioned this pull request Apr 28, 2026

Track array value type overwrites in by-reference foreach on constant arrays #5556

Closed

VincentLanglet added a commit to phpstan-bot/phpstan-src that referenced this pull request Apr 28, 2026

Reapply "Track array value type overwrites in by-reference foreach …

a5f0c72

…without key variable (phpstan#5542)" This reverts commit 6c69c44.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track array value type overwrites in by-reference `foreach` without key variable#5542

Track array value type overwrites in by-reference `foreach` without key variable#5542
VincentLanglet merged 4 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-vwbkpoi

phpstan-bot commented Apr 26, 2026

Uh oh!

VincentLanglet left a comment

Uh oh!

phpstan-bot commented Apr 26, 2026

Uh oh!

VincentLanglet Apr 27, 2026

Uh oh!

phpstan-bot Apr 27, 2026

Uh oh!

VincentLanglet left a comment

Uh oh!

phpstan-bot commented Apr 27, 2026

Uh oh!

VincentLanglet left a comment

Uh oh!

phpstan-bot commented Apr 27, 2026

Uh oh!

staabm left a comment •

edited

Loading

Uh oh!

Uh oh!

ondrejmirtes commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

phpstan-bot commented Apr 26, 2026

Summary

Changes

Root cause

Test

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

phpstan-bot commented Apr 26, 2026

Uh oh!

VincentLanglet Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

phpstan-bot Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

phpstan-bot commented Apr 27, 2026

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

phpstan-bot commented Apr 27, 2026

Uh oh!

staabm left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ondrejmirtes commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

staabm left a comment •

edited

Loading